-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Cleanup rustdoc themes #100494
Cleanup rustdoc themes #100494
Conversation
A change occurred in the Ayu theme. cc @Cldfire |
/* Without the `!important`, the border-color is ignored for `<select>`... | ||
It cannot be in the group above because `.search-input` has a different border color on | ||
hover. */ | ||
border: 1px solid var(--border-color) !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand this comment. I think border: 1px solid var(--border-color)
should work.
Are you familiar with how CSS Specificity works? Most instances of !important
can be better solved by applying the principles of specificity.
Also, when you say "cannot be in the group above", (a) you should say "in the rule above" or "in the ruleset above" (https://developer.mozilla.org/en-US/docs/Web/CSS/Syntax#css_rulesets). Using precise language helps clarify. Also: are you talking about the ruleset with the selector #crate-search-div
? I don't see how that one is relevant.
e4d3d38
to
a27f574
Compare
This comment has been minimized.
This comment has been minimized.
a27f574
to
7f79857
Compare
I also updated the online demo. |
7f79857
to
47edd4f
Compare
Added the border everywhere and removed it from ayu theme. |
@@ -138,6 +138,9 @@ h1, h2, h3, h4 { | |||
.docblock h3, .docblock h4, h5, h6 { | |||
margin: 15px 0 5px 0; | |||
} | |||
.docblock table td, .docblock table th { | |||
border-color: var(--border-color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be combined with the rules that set the border for these elements in the first place.
.docblock table td {
padding: .5em;
border: 1px dashed;
}
.docblock table th {
padding: .5em;
text-align: left;
border: 1px solid;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes absolutely. Don't know how I didn't think about it...
} | ||
#settings-menu > a, #help-button > button, #copy-path { | ||
padding: 5px; | ||
width: 33px; | ||
border: 1px solid; | ||
border-radius: 2px; | ||
cursor: pointer; | ||
border-color: var(--border-color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be combined with the border
property on line 1543 above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm quite ashamed of this one...
@@ -1539,6 +1552,7 @@ pre.rust { | |||
padding: 5px; | |||
height: 100%; | |||
display: block; | |||
background-color: var(--button-background-color); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is not needed. It's redundant with the ruleset at line 1540.
@@ -840,11 +848,11 @@ nav.main { | |||
text-align: center; | |||
} | |||
nav.main .current { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this ruleset entirely. We don't currently generate a nav.main .current
AFAICT.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't seem to have a nav.main
indeed (but we do have .current
) so this rule is unused.
47edd4f
to
09396fc
Compare
Applied suggestions and updated live demo. |
@bors r+ rollup |
Rollup of 9 pull requests Successful merges: - rust-lang#99576 (Do not allow `Drop` impl on foreign fundamental types) - rust-lang#100081 (never consider unsafe blocks unused if they would be required with deny(unsafe_op_in_unsafe_fn)) - rust-lang#100208 (make NOP dyn casts not require anything about the vtable) - rust-lang#100494 (Cleanup rustdoc themes) - rust-lang#100522 (Only check the `DefId` for the recursion check in MIR inliner.) - rust-lang#100592 (Manually implement Debug for ImportKind.) - rust-lang#100598 (Don't fix builtin index when Where clause is found) - rust-lang#100721 (Add diagnostics lints to `rustc_type_ir` module) - rust-lang#100731 (rustdoc: count deref and non-deref as same set of used methods) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
…r=notriddle Cleanup css theme Follow-up of rust-lang#100494. The change for the border color of the search input in the dark mode was actually a weird case: the search input border was unique, it didn't share the same variable with other items with borders. This weird case being unique to the dark theme, I removed it, hence the modification in the GUI test. Live demo is [here](https://rustdoc.crud.net/imperio/cleanup-css-theme/std/index.html). cc `@jsha` r? `@notriddle`
This PR continues our work to simplify the rustdoc themes by relying more on CSS variables. Interestingly enough, this time it allowed me to realize that we were having a lot of different colors for borders even though the difference is unnoticeable. I used this opportunity to unify them.
Follow-up of #98460.
The live demo is here.
r? @jsha